-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix #3906 by forcing Record constructors to be visible regardless of visibility overrides #3944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@yihtserns WDYT? |
As noted by this comment, private canonical constructor (i.e. for Do we want to change |
(Sorry for going MIA with regards to #3906 - besides day job, I was under the impression that it wasn't urgent so I've been researching for a different way to fix that) |
@yihtserns Hmmh. Good question. I don't know to be sure; perhaps? But I also noticed that there's one regression here, for And NP for no follow up: I agree that this is not the most important thing to tackle. And with the issues there are maybe another approach would be useful. |
Hi, Can you please fix this problem? I think this is the reason for some troubles that we have been having. |
Unfortunately this breaks some tests. So someone with time should figure out what the problem is, and whether this fix is actually safe to apply or not. I can't (and shouldn't) fix in PR that breaks existing tests. @CatarinaRibas perhaps you have time to dig into this to help? Discussion on #3906 side may be a good place to continue discussions on people involved; I created PR but am not most knowledgeable here (nor have time right now) |
6b441ed
to
018372e
Compare
@cowtowncoder looks the failures are related to this. the expected result for that is causing a single test (based around that specific disabled AUTO_DETECT_CREATORS case) to fail. i got the tests to pass when i changed the condition in the new code to ClassUtil.isRecordType(baseType) && isEnabled(MapperFeature.AUTO_DETECT_CREATORS) |
Superceded by #4057 (targeting 2.16, too risky for patch release) |
Fixes #3906 as per title.